Add Test Fixtures to separate test code from production code #6904
Conversation
jessicaKoehnke
added
the
+Skip Changelog
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev-2.x #6904 +/- ##
=============================================
- Coverage 72.10% 72.09% -0.01%
+ Complexity 19663 19658 -5
=============================================
Files 2123 2124 +1
Lines 79428 79509 +81
Branches 8049 8051 +2
=============================================
+ Hits 57269 57324 +55
- Misses 19319 19347 +28
+ Partials 2840 2838 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
jessicaKoehnke
added
the
!Technical Debt
| FileType type, | ||
| DataSource... children | ||
| ) { | ||
| return new ListCompositeDataSource(name, type, Arrays.asList(children)); |
There was a problem hiding this comment.
The ListCompositeDataSource can also be moved here because it is not used in production - and I do not see that it would be relevant for production. If so, we could move it back. Not needed for me to approve PR.
| /** | ||
| * Use this to get a composite data source, bypassing the {@link OtpDataStore}. | ||
| */ | ||
| public static CompositeDataSource compositeSource(File file, FileType type) { |
There was a problem hiding this comment.
I would love to deprecate this to simplify tests, the next method is much easier to use.
|
|
||
| public class GtfsBundleTestFactory { | ||
|
|
||
| public static GtfsBundle forTest(File gtfsFile, @Nullable String feedId) { |
There was a problem hiding this comment.
Seems that prettier is not configured to format these files. Can you change that?
leonardehrenfried
left a comment
leonardehrenfried
left a comment
There was a problem hiding this comment.
Please configure prettier.
|
|
|
I only moved a few classes to demonstrate how it works. I can move more ( |
|
I would be fine with both. It's up to you. |
4 participants
Summary
This is a suggestion on how we could better separate test code from production code.
Issue
We have a lot of
forTestmethods all over our production code. This could potentially lead to code intended for tests to accidentally be used in production. In my opinion test code and production code should never be mixed.This solution is inspired by the gradle testFixtures plugin. I added a new source to the application module to more or less achieve the same result as the gradle plugin. All classes defined in the new test-fixtures source can be used in all test sources, but not in the production code sources.
To demonstrate the usage I moved the test data factories touched by #6840 into the new test-fixtures source.
Unit tests
no code added -> no tests needed
Documentation
If we decide to move forward with this, it might be a good idea to update the developer guidelines.